fix(sandbox): skip read-only mounts during recursive chown of /sandbox#2341
Open
varshaprasad96 wants to merge 3 commits into
Open
fix(sandbox): skip read-only mounts during recursive chown of /sandbox#2341varshaprasad96 wants to merge 3 commits into
varshaprasad96 wants to merge 3 commits into
Conversation
varshaprasad96
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
July 17, 2026 05:55
The sandbox supervisor crashed with EROFS when the recursive chown of /sandbox encountered read-only submounts. This is common in gVisor-based Kubernetes deployments where read-only volume mounts are the only way to enforce per-directory immutability (Landlock is unavailable under gVisor). The fix adds two guards to the ownership walk: 1. Mount-boundary detection via st_dev comparison — paths on a different filesystem than /sandbox are skipped entirely, avoiding the chown call on nested read-only mounts. 2. EROFS tolerance — if chown still returns EROFS (e.g. the root mount itself is read-only), the error is logged at debug level and startup continues. Symlink skipping (already present) is preserved and extracted into the recursive walker for consistency. Manually verified on a kind cluster with a read-only PVC mounted at /sandbox/readonly-data: the pod starts successfully, writable paths are owned by the sandbox user, and the read-only mount retains root ownership. Kubernetes e2e test coverage is a separate follow-up. Closes NVIDIA#2294 Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
varshaprasad96
force-pushed
the
fix/sandbox-erofs-readonly-mounts
branch
from
July 17, 2026 06:00
6dadfae to
90fae13
Compare
elezar
reviewed
Jul 17, 2026
Comment on lines
+1197
to
+1198
| let meta = std::fs::symlink_metadata(path).into_diagnostic()?; | ||
|
|
Member
There was a problem hiding this comment.
Can we move the symlink check that is currently performed on child in the loop below to here?
Contributor
Author
There was a problem hiding this comment.
Done in 4cd9059. Moved the symlink check into the top of chown_recursive (reusing the symlink_metadata call already there), which also eliminated the redundant per-child symlink_metadata in the loop
Move the per-child symlink guard from the directory iteration loop into the top of chown_recursive, reusing the symlink_metadata call that is already performed there. This centralizes all three skip guards (symlink, mount boundary, EROFS) in one place and eliminates a redundant symlink_metadata call per child entry. Suggested-by: elezar Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
When chown returns EROFS on a directory, the code previously returned early without recursing into children. This skipped writable submounts nested under a read-only directory on the same device (e.g., a writable emptyDir inside a read-only ConfigMap mount sharing the same st_dev). Now the EROFS case falls through to the directory recursion so that writable children are still chowned. Also fixes the doc comment which said EROFS errors are "logged as warnings" when they are actually logged at debug level. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix sandbox supervisor crash (
EROFS: Read-only file system) when the recursivechown /sandboxencounters read-only submounts. This is common in gVisor-based Kubernetes deployments where read-only volume mounts enforce per-directory immutability (Landlock is unavailable under gVisor).Related Issue
Closes #2294
Changes
st_devof each path against the root/sandboxdevice — paths on a different filesystem are skipped entirely, avoiding thechowncall on nested read-only mountschownstill returnsEROFS(e.g. the root mount itself is read-only), the error is logged at debug level and startup continueschown_sandbox_home: Split into entry-point (symlink guard + root_dev capture) andchown_recursive(inner walk with st_dev and EROFS guards)chown_recursive_skips_different_deviceandchown_sandbox_home_does_not_chown_across_device_boundary; strengthened existing symlink rejection testTesting
mise run pre-commitpasses (Rust lint/format/clippy clean; unrelated Python proto env issue)/sandbox/readonly-data: pod starts successfully, writable paths owned by sandbox user, read-only mount retains root ownershipChecklist